public API overhaul #647
public API overhaul #647
Conversation
🦋 Changeset detectedLatest commit: 234e521 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| @@ -85,11 +85,14 @@ impl TestNodeBuilder { | |||
| } | |||
|
|
|||
| impl TestNode { | |||
| pub fn from_cst(node: &Node) -> Self { | |||
| pub fn from_cst(cursor: Cursor) -> Self { | |||
There was a problem hiding this comment.
You could use mut cursor here
| @@ -93,7 +93,7 @@ impl Match { | |||
| pub fn is_full_recursive(&self) -> bool { | |||
| self.nodes | |||
| .iter() | |||
| .flat_map(cst::Node::cursor) | |||
| .flat_map(|node| cst::Node::create_cursor(node, Default::default())) | |||
There was a problem hiding this comment.
I'm not a fan of using the Default as parameter and think it'd be clearer if we have a separate cursor_without_offset (without the argument) function cursor_with_offset (or something along the lines).
This is fine for now, but I'd rethink how we can design the API to better convey that only the root node cursor has absolute offsets and other created ones are relative.
There was a problem hiding this comment.
I think cursor_without_offset() should create a cursor that doesn't track offsets at all (for perf, and also to not expose an invalid .offset property).
Also I thought about introducing a TextIndex::ZERO helper.
But I think we should consider this as part of future updates to the API. I want to keep this PR minimal to address existing feedback.
| @@ -18,4 +18,8 @@ impl ParseOutput { | |||
| pub fn is_valid(&self) -> bool { | |||
| return self.errors.is_empty(); | |||
| } | |||
|
|
|||
| pub fn create_tree_cursor(&self) -> Cursor { | |||
There was a problem hiding this comment.
If exposing the API is important for us now, I would add more documentation to the public cursor functions that the user might use
OmarTawfik
force-pushed
the
public-api-overhaul
branch
from
48d8fed
to
234e521
Compare
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and publish to npm yourself or [setup this action to publish automatically](https://github.com/changesets/action#with-publishing). If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @nomicfoundation/slang@0.11.0 ### Minor Changes - [#625](#625) [`7bb650b`](7bb650b) Thanks [@Xanewok](https://github.com/Xanewok)! - The CST Cursor now implements the Iterator trait as part of the Rust API - [#647](#647) [`b1dced3`](b1dced3) Thanks [@OmarTawfik](https://github.com/OmarTawfik)! - Require specifying an initial offset when creating a CST cursor. ### Patch Changes - [#648](#648) [`2327bf5`](2327bf5) Thanks [@OmarTawfik](https://github.com/OmarTawfik)! - Support Solidity v0.8.22. - [#623](#623) [`80114a8`](80114a8) Thanks [@AntonyBlakey](https://github.com/AntonyBlakey)! - Correct the types in the TS api by adding the correct namespaces to type references Co-authored-by: OmarTawfik <15987992+OmarTawfik@users.noreply.github.com>
with_colourtowith_coloras we are using a singleen-USlocale in the entire project.SUPPORTED_VERSIONS.binary_search()inLanguage::new()